chore(core): remove dead code, unnecessary casts, and redundant async by deyaaeldeen · Pull Request #38149 · Azure/azure-sdk-for-js

deyaaeldeen · 2026-04-15T00:37:12Z

Summary

Remove dead code, unnecessary type casts, and redundant async keywords across core packages.

Dead code removal

AsyncLockOptions interface, Timeout class (core-amqp)
isBearerToken, isPopToken helpers (core-auth)
getCachedDefaultHttpsClient, unused HttpClient import (core-client-rest)
Stale declare global { interface Event {} } (abort-controller)

Cast removal / type narrowing

as any → as ParsedOutput in parseConnectionString (core-amqp)
{ [k: string]: string } → Record (core-amqp)
as string → ?? "" for CAE challenge header (core-rest-pipeline)
isKeyCredential param any → unknown with proper guard (core-client-rest)
objectHasProperty tightened with null check (core-util)

Redundant async removal

sendRequest, sendPolicy handlers that just return a promise without awaiting (core-client, core-client-rest, core-http-compat, core-lro)

Simplify defensive logic

ndJsonPolicy: Remove unreachable Array.isArray guard after JSON.parse on a string starting with [ (JSON spec guarantees array result)
tokenCycler: Replace optional chaining with explicit null guard in shouldRefresh, making intent clearer

Other improvements

Resolved simplified to Awaited with deprecation notice (core-tracing)
HttpHeaders class made non-exported (core-http-compat, was already not in public API)
Set for passthrough props lookup (core-http-compat)
XML root key extraction simplified (core-xml)
URL scope construction fix (core-client)
Optional chaining removed on guaranteed field (core-paging)

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Removes dead code, redundant async usage, and unnecessary casts/type aliases across core packages, aiming to simplify implementations and tighten typing.

Changes:

Removed unused helpers/types and stale declarations across multiple core packages.
Simplified type handling (e.g., Resolved<T> → Awaited<T>, narrower type guards, fewer casts).
Removed redundant async wrappers that only forwarded promises and made small micro-optimizations (e.g., Set membership).

Reviewed changes

Copilot reviewed 22 out of 23 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
sdk/core/core-xml/src/xml.ts	Simplifies root key extraction when `includeRoot` is false.
sdk/core/core-util/src/typeGuards.ts	Tightens `objectHasProperty` implementation with explicit null check.
sdk/core/core-tracing/src/tracingClient.ts	Replaces `Resolved` with `Awaited`, removes redundant `Promise.resolve`, and simplifies object literals.
sdk/core/core-tracing/src/interfaces.ts	Deprecates `Resolved` in favor of built-in `Awaited`.
sdk/core/core-tracing/review/core-tracing-node.api.md	Updates public API surface to mark `Resolved` deprecated and uses `Awaited`.
sdk/core/core-rest-pipeline/src/policies/bearerTokenAuthenticationPolicy.ts	Removes unsafe cast on `WWW-Authenticate` header retrieval.
sdk/core/core-paging/src/getPagedAsyncIterator.ts	Removes optional chaining on a guaranteed `pagedResult` reference.
sdk/core/core-lro/src/poller/poller.ts	Removes redundant `async` from progress event handler.
sdk/core/core-lro/src/http/operation.ts	Removes redundant `async` wrapper around `lro.sendPollRequest`.
sdk/core/core-http-compat/src/util.ts	Replaces passthrough prop list with a `Set`; makes `HttpHeaders` non-exported.
sdk/core/core-http-compat/src/policies/disableKeepAlivePolicy.ts	Removes redundant `async` from policy `sendRequest`.
sdk/core/core-client/src/urlHelpers.ts	Refactors URL path/search scope construction.
sdk/core/core-client/src/serviceClient.ts	Removes redundant `async` in `sendRequest`; adjusts credential scope error condition.
sdk/core/core-client/src/serializationPolicy.ts	Removes redundant `async` from policy `sendRequest`.
sdk/core/core-client/src/authorizeRequestOnTenantChallenge.ts	Fixes scope URL construction using `URL` base resolution.
sdk/core/core-client-rest/src/restError.ts	Replaces non-null assertion with safer cast for forwarding to tsp helper.
sdk/core/core-client-rest/src/keyCredentialAuthenticationPolicy.ts	Removes redundant `async` from policy `sendRequest`.
sdk/core/core-client-rest/src/clientHelpers.ts	Removes cached HTTP client dead code; tightens `isKeyCredential` parameter type.
sdk/core/core-client-rest/src/apiVersionPolicy.ts	Simplifies query param presence check using `searchParams.size`.
sdk/core/core-auth/src/tokenCredential.ts	Removes unused internal token-type helper functions.
sdk/core/core-amqp/src/util/utils.ts	Removes dead types/helpers; tightens `parseConnectionString` return typing.
sdk/core/abort-controller/src/index.ts	Removes stale global `Event` declaration.

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

Copilot

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated 5 comments.

- Remove unused AsyncLockOptions, Timeout class (core-amqp) - Remove unused isBearerToken, isPopToken helpers (core-auth) - Remove unused getCachedDefaultHttpsClient (core-client-rest) - Remove stale global Event declaration (abort-controller) - Replace `as any` casts with proper type narrowing - Remove unnecessary `async` on non-awaiting functions - Simplify Resolved<T> to Awaited<T> (core-tracing) - Use Set for passthrough props lookup (core-http-compat) - Make HttpHeaders class non-exported (core-http-compat) - Simplify XML root key extraction (core-xml) - Fix URL scope construction (core-client) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

xirzec

Mostly good, though I have some questions.

- Restore async on handleProgressEvents to preserve microtask timing - Restore original objectHasProperty (no function support needed) - Remove associated function tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- ndJsonPolicy: Remove unreachable Array.isArray guard after JSON.parse on a string starting with '[' (JSON spec guarantees array result) - tokenCycler: Replace optional chaining with explicit null guard in shouldRefresh, making intent clearer Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

The ndJsonPolicy and tokenCycler simplifications have been moved to the chore/core-src-cleanup branch (PR #38149). This PR now contains only test additions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

xirzec

Pretty sure the tokenCycler change is incorrect

The public API re-exports calculateRetryDelay from index.ts which delegates to @typespec/ts-http-runtime. The implementation in delay.ts was never called. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

randomNumberFromInterval, executePromisesSequentially, isIotHubConnectionString, and Timeout were removed in #38149. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…#38149) ## Summary Remove dead code, unnecessary type casts, and redundant `async` keywords across core packages. ### Dead code removal - `AsyncLockOptions` interface, `Timeout` class (core-amqp) - `isBearerToken`, `isPopToken` helpers (core-auth) - `getCachedDefaultHttpsClient`, unused `HttpClient` import (core-client-rest) - Stale `declare global { interface Event {} }` (abort-controller) ### Cast removal / type narrowing - `as any` → `as ParsedOutput` in `parseConnectionString` (core-amqp) - `{ [k: string]: string }` → `Record` (core-amqp) - `as string` → `?? ""` for CAE challenge header (core-rest-pipeline) - `isKeyCredential` param `any` → `unknown` with proper guard (core-client-rest) - `objectHasProperty` tightened with null check (core-util) ### Redundant async removal - `sendRequest`, `sendPolicy` handlers that just return a promise without awaiting (core-client, core-client-rest, core-http-compat, core-lro) ### Simplify defensive logic - **ndJsonPolicy**: Remove unreachable `Array.isArray` guard after `JSON.parse` on a string starting with `[` (JSON spec guarantees array result) - **tokenCycler**: Replace optional chaining with explicit null guard in `shouldRefresh`, making intent clearer ### Other improvements - `Resolved` simplified to `Awaited` with deprecation notice (core-tracing) - `HttpHeaders` class made non-exported (core-http-compat, was already not in public API) - `Set` for passthrough props lookup (core-http-compat) - XML root key extraction simplified (core-xml) - URL scope construction fix (core-client) - Optional chaining removed on guaranteed field (core-paging) --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Deyaaeldeen Almahallawi <deyaa@microsoft.com>

randomNumberFromInterval, executePromisesSequentially, isIotHubConnectionString, and Timeout were removed in #38149. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

deyaaeldeen added the Azure.Core label Apr 15, 2026

Copilot AI review requested due to automatic review settings April 15, 2026 00:37

deyaaeldeen requested review from a team, jeremymeng, joheredi, maorleger, timovv and xirzec as code owners April 15, 2026 00:37

Copilot AI reviewed Apr 15, 2026

View reviewed changes

Comment thread sdk/core/core-client/src/urlHelpers.ts Outdated

Comment thread sdk/core/core-client/src/urlHelpers.ts Outdated

Comment thread sdk/core/core-client/src/urlHelpers.ts Outdated

Comment thread sdk/core/core-client-rest/src/clientHelpers.ts Outdated

deyaaeldeen force-pushed the chore/core-src-cleanup branch from 04d2aef to 6ceaa01 Compare April 15, 2026 01:09

Copilot started reviewing on behalf of deyaaeldeen April 15, 2026 01:36 View session

deyaaeldeen force-pushed the chore/core-src-cleanup branch 2 times, most recently from 9ec70fd to 580cea9 Compare April 15, 2026 02:33

deyaaeldeen requested a review from Copilot April 15, 2026 02:36

Copilot AI reviewed Apr 15, 2026

View reviewed changes

Comment thread sdk/core/core-util/src/typeGuards.ts Outdated

Comment thread sdk/core/core-client-rest/src/restError.ts

Comment thread sdk/core/core-tracing/src/interfaces.ts

Comment thread sdk/core/core-http-compat/src/util.ts

Comment thread sdk/core/core-xml/CHANGELOG.md

Copilot started reviewing on behalf of deyaaeldeen April 15, 2026 02:44 View session

deyaaeldeen force-pushed the chore/core-src-cleanup branch from 580cea9 to 4ea1fd2 Compare April 15, 2026 02:46

deyaaeldeen force-pushed the chore/core-src-cleanup branch from 4ea1fd2 to d2d2cf8 Compare April 15, 2026 16:59

xirzec reviewed Apr 15, 2026

View reviewed changes

Comment thread sdk/core/core-client-rest/CHANGELOG.md

xirzec reviewed Apr 15, 2026

View reviewed changes

Comment thread sdk/core/core-amqp/src/util/utils.ts

Comment thread sdk/core/core-lro/src/poller/poller.ts Outdated

Comment thread sdk/core/core-tracing/src/interfaces.ts

Comment thread sdk/core/core-util/src/typeGuards.ts Outdated

deyaaeldeen force-pushed the chore/core-src-cleanup branch from d2d2cf8 to 71e3db9 Compare April 15, 2026 20:11

deyaaeldeen force-pushed the chore/core-src-cleanup branch from 71e3db9 to 6a684ca Compare April 15, 2026 20:13

deyaaeldeen requested a review from xirzec April 15, 2026 21:16

deyaaeldeen and others added 2 commits April 15, 2026 21:43

add changelog entry for deprecated Resolved<T> type

4132e8b

deyaaeldeen mentioned this pull request Apr 16, 2026

Improve test coverage to ~100% across all core libraries #38161

Closed

xirzec reviewed Apr 16, 2026

View reviewed changes

Comment thread sdk/core/core-rest-pipeline/src/policies/ndJsonPolicy.ts

Comment thread sdk/core/core-rest-pipeline/src/util/tokenCycler.ts Outdated

Deyaaeldeen Almahallawi and others added 2 commits April 16, 2026 17:25

shouldRefresh return true if token is null

11dbf92

This was referenced Apr 17, 2026

Improve test coverage to ~100% for @azure/core-auth #38175

Closed

Improve test coverage to ~100% for @azure/core-util #38182

Merged

xirzec approved these changes Apr 17, 2026

View reviewed changes

deyaaeldeen merged commit 8edb91b into main Apr 17, 2026
17 checks passed

deyaaeldeen deleted the chore/core-src-cleanup branch April 17, 2026 23:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(core): remove dead code, unnecessary casts, and redundant async#38149

chore(core): remove dead code, unnecessary casts, and redundant async#38149
deyaaeldeen merged 6 commits intomainfrom
chore/core-src-cleanup

deyaaeldeen commented Apr 15, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xirzec left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xirzec left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

deyaaeldeen commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Dead code removal

Cast removal / type narrowing

Redundant async removal

Simplify defensive logic

Other improvements

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xirzec left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xirzec left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

deyaaeldeen commented Apr 15, 2026 •

edited

Loading